Skip to content

tail: avoid unnecessary pipe2 syscall for empty files#12465

Open
dragutreis wants to merge 2 commits into
uutils:mainfrom
dragutreis:fix/tail-is-file-exhausted
Open

tail: avoid unnecessary pipe2 syscall for empty files#12465
dragutreis wants to merge 2 commits into
uutils:mainfrom
dragutreis:fix/tail-is-file-exhausted

Conversation

@dragutreis

@dragutreis dragutreis commented May 24, 2026

Copy link
Copy Markdown
Contributor

Fixes #12464

bounded_tail calls print_target_section unconditionally, which creates
a broker pipe via splice_unbounded_broker even when the file is empty or
exhausted, causing an unnecessary pipe2 syscall.

Added is_file_exhausted to check before calling print_target_section:

  • Regular files: compare stream position against file length
  • Char/block devices: probe with a single-byte read; if data exists, write
    it directly to stdout since char devices don't support seeking back

Before:
strace -c -e pipe,pipe2 tail -c +1 /dev/null → 1 pipe2 call
After:
strace -c -e pipe,pipe2 tail -c +1 /dev/null → 0 pipe2 calls

@oech3

oech3 commented May 24, 2026

Copy link
Copy Markdown
Contributor

Does this require additional stat or other syscall? I'm not sure which syscall is cheap, but I thins this is not needed considering usage for 0-sized file is very rare.

@dragutreis

dragutreis commented May 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Compared full syscall counts with strace -c:

Empty file (/dev/null):

  • Without fix: 129 syscalls (includes pipe2 + splice)
  • With fix: 124 syscalls

No new syscalls added. statx goes down by 1.

Non-empty file:

  • Without fix: 124 syscalls
  • With fix: 124 syscalls

No overhead for the common case.

@github-actions

github-actions Bot commented May 24, 2026 •
edited
Loading

Copy link
Copy Markdown

GNU testsuite comparison:

Skip an intermittent issue tests/tail/retry (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/cut/bounded-memory (passes in this run but fails in the 'main' branch)
Note: The gnu test tests/expand/bounded-memory is now being skipped but was previously passing.

@oech3

oech3 commented May 24, 2026

Copy link
Copy Markdown
Contributor

it directly to stdout since char devices don't support seeking back

Are /dev/sd* and /dev/nvme* missing splice driver yet? I saw cat does not call splice yet. (Though I want to cover the case kernel started supporting someday).

@dragutreis

dragutreis commented May 24, 2026

Copy link
Copy Markdown
Contributor Author

it directly to stdout since char devices don't support seeking back

Are /dev/sd* and /dev/nvme* missing splice driver yet? I saw cat does not call splice yet. (Though I want to cover the case kernel started supporting someday).

tested on /dev/nvme0n1, splice is called successfully, so block devices do support it

sylvestre
sylvestre reviewed May 24, 2026
View reviewed changes
Comment thread src/uu/tail/src/tail.rs Outdated
/// For regular files, checks position against file length.
/// For char/block devices, probes by reading one byte; if data exists, writes
/// it to `out` so it isn't lost (char devices don't support seeking back).
#[allow(unused_variables)]

@sylvestre sylvestre May 24, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please remove this allow

@dragutreis dragutreis May 24, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

alright

@oech3

oech3 commented May 26, 2026

Copy link
Copy Markdown
Contributor

Would you add test for char/block devises with 2 byte input?

@codspeed-hq

codspeed-hq Bot commented May 26, 2026

Copy link
Copy Markdown

Merging this PR will create unknown performance changes

⚠️ No benchmarks were detected in both the base of the PR and the PR.
Please ensure that your benchmarks are correctly instrumented with CodSpeed.

Check out the benchmarks creation guide

Comparing dragutreis:fix/tail-is-file-exhausted (9794752) with main (c8af53a)

Open in CodSpeed

@oech3

oech3 commented May 26, 2026

Copy link
Copy Markdown
Contributor

It would be better to include read/write path for small input to splice modules for all utilities instead of patching tail only. Then we only pay 0 byte read at here. I'll do that soon.

@oech3

oech3 commented Jun 1, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

If I am understanding correctly, unnecessary pipe2 call is already fixed at different your PR.
Also 0-sized file should just finish at 1st splice's EOF and usecase for 0-sized file is very very rare. Is this worth to optimize this with additional complexity?

@oech3

oech3 commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

#12548 should reduce some unnecessary splice calls.

@oech3 oech3 mentioned this pull request Jun 5, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sylvestre sylvestre sylvestre left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

tail: unnecessary pipe2 syscall for empty files

3 participants

@dragutreis @oech3 @sylvestre